fix(workflows): Fix empty results in Workflows query type #526
Conversation
Fixes grafana#503 - Add nil check for CreatedAt/UpdatedAt timestamps to prevent panics - Add 'None' option to Time Field dropdown (default) to return all workflows - Improve time filtering logic with better nil handling - Add debug logging for troubleshooting
|
|
|
|
zoltanbedi
left a comment
zoltanbedi
left a comment
There was a problem hiding this comment.
Thanks for creating this PR @idan-starkware. I tried it out and it worked as expected. Please create a changeset for this and also update the docs in docs/sources/query/_index.md.
| { | ||
| "name": "grafana-github-datasource", | ||
| "version": "2.3.0", | ||
| "version": "2.3.1", |
There was a problem hiding this comment.
Can you please remove this? We will release a new version with this change in a separate pr.
|
|
||
| // If time field is None, return all workflows without filtering | ||
| if opts.TimeField == models.WorkflowTimeFieldNone { | ||
| backend.Logger.Debug("GetWorkflows", "time_field_none", "returning_all_workflows") |
There was a problem hiding this comment.
We can figure it out from the debug log above that the time field is none.
| backend.Logger.Debug("GetWorkflows", "time_field_none", "returning_all_workflows") |
| shouldInclude = !createdAtTime.Before(timeRange.From) && !createdAtTime.After(timeRange.To) | ||
| if !shouldInclude { | ||
| excludedCount++ | ||
| backend.Logger.Debug("keepWorkflowsInTimeRange", "workflow_excluded", *workflow.Name, "createdAt", createdAtTime, "timeRange", fmt.Sprintf("%v to %v", timeRange.From, timeRange.To)) |
There was a problem hiding this comment.
Please remove this debug log. The summary debug log is enough.
| shouldInclude = !updatedAtTime.Before(timeRange.From) && !updatedAtTime.After(timeRange.To) | ||
| if !shouldInclude { | ||
| excludedCount++ | ||
| backend.Logger.Debug("keepWorkflowsInTimeRange", "workflow_excluded", *workflow.Name, "updatedAt", updatedAtTime, "timeRange", fmt.Sprintf("%v to %v", timeRange.From, timeRange.To)) |
There was a problem hiding this comment.
Please remove this debug log. The summary debug log is enough.
3 participants
Fixes #503
Testing